New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add kubectl-convert to client-binaries #99155
Conversation
/triage accepted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I typically would not prefer to add a binary to our release artifacts that has already been removed from kubectl
because of its dependencies. However, I also understand that kubectl-convert
is pretty tightly coupled to k8s versions. I wonder how we feel about this compared to something like kubeadm
, which has seen some motivation to be moved out of k/k. kubectl-convert
is difficult because the same reason it was broken out from kubectl
(dependency on internal types) seems to be the primary motivation for including it in k/k release process. Would like to discuss more before we commit to releasing as part of k/k because we are trying to minimize churn on the artifacts we are producing.
/hold
while we had plans to move kubeadm out of k/k the preference was to still continue releasing it as part of the k8s bundle.
i'm +1 on including kubectl-convert in the release, or if not kubectl-convert, a general mechanism to allow users to migrate their manifests on disk away from a unsupported API. |
Another thing to note is that we should actively be testing artifacts that are part of the k/k release bundles. Do we have periodic jobs exercising |
+1 to @hasheddan's assessment. Will hold any comments from my side pending response. |
Pairing That said, if the goal of
+1 to testing. |
We used to have some test in test-cmd suite, lemme ensure they are properly testing |
f612870
to
07f34e6
Compare
Now with convert test in 2nd commit. |
/approve (/build and /hack changes look good) |
/retest |
(Will drop an |
07f34e6
to
5681a58
Compare
Fixed all the bits, should be good now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/hold cancel
Acknowledging that this is a special case and testing has been added.
/retest
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dims, justaugustus, soltysh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Relevant to kubernetes/website#28724: Document how to install kubectl convert plugin |
What type of PR is this?
/kind cleanup
/kind documentation
/sig cli
/sig release
/priority backlog
What this PR does / why we need it:
In #96190 I moved all the remaining bits of kubectl to staging repository. Per kubernetes/kubectl#725 it was decided that
kubectl convert
will become a plugin but I missed it in that PR. This should fix our release such that it will includekubectl-convert
binaries alongkubectl
.Which issue(s) this PR fixes:
Fixes #99076
Special notes for your reviewer:
/assign @liggitt @neolit123
Does this PR introduce a user-facing change?